-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix compilation errors if only the feature hash2curve is enabled #996
fix compilation errors if only the feature hash2curve is enabled #996
Conversation
p256/src/arithmetic/hash2curve.rs
Outdated
@@ -93,7 +93,9 @@ impl FromOkm for Scalar { | |||
|
|||
#[cfg(test)] | |||
mod tests { | |||
use crate::{arithmetic::field::MODULUS, FieldElement, NistP256, Scalar, U256}; | |||
#![cfg(feature = "expose-field")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#![cfg(feature = "expose-field")] | |
#[cfg(feature = "expose-field")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, that increased the diff a bit, but made it more correct I think. Thanks for the feedback
p256/tests/ecdsa.rs
Outdated
@@ -1,6 +1,6 @@ | |||
//! ECDSA tests. | |||
|
|||
#![cfg(feature = "arithmetic")] | |||
#![cfg(all(feature = "arithmetic", feature = "ecdsa-core"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this related to hash2curve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reworked this a bit, it was an error that showed up after I fixed the one in the hash2curve.rs file. Hopefully more correct now.
6af2252
to
6f01310
Compare
I took a slightly different approach in #998 |
Merged #996 instead which fixes the tests and adds them to CI |
That was a much better approach, thanks :) |
Without this patch: